[Server] Wire up notifications for changing prompt, resource & tools lists#234
[Server] Wire up notifications for changing prompt, resource & tools lists#234chr-hertel wants to merge 1 commit intomainfrom
Conversation
ad9b937 to
704dc13
Compare
|
I'm working on an MCP server that dynamically registers tools at runtime (tools are discovered progressively as the user navigates an API), so I'm hitting exactly the two problems you describe. Wanted to share what I ended up doing. Notification delivery via Fiber::suspend()I don't use the event system — I use // After registering N tools in a loop:
if ($newToolsRegistered) {
\Fiber::suspend([
'type' => 'notification',
'notification' => new ToolListChangedNotification(),
]);
}This already works with the current SDK and has the advantage of batching — one notification after registering all discovered tools, rather than one event per Worth considering: the event-based approach and the Fiber::suspend approach need to coexist cleanly. If Session state (your TBD #2)This was the hardest part for me too. I needed session access outside of the handler to persist tool state across HTTP requests. What I ended up doing:
Storing session on Protocol (your current approach) makes sense as the central place, but I'd flag the Fiber concern — if Thoughts on the PR
Related: #251 — I also ran into Note: LLM generated comment based on my work I hope you're fine with it I thought it'd be nice to share my solutions :) |
This is a working draft for emitting change notifications on dynamically added prompts, resources and tools.
The main issue here is that we need that indirection between Registry, source of truth on changing lists, and Protocol, that is able so send notifications. That's why we have those events, but we never really adopted them.
I introduced a slim EventDispatcher/ListenerProvider setup, but it feels a bit flaky - see wire up in example. The service landscape within the SDK just grows in complexity.
TBD
Event Dispatcher
The issue I had was mostly about PSR-14 - we want to empower library users to inject their own event dispatcher, to easily register subscriber/listener/whatever - but with the current implementation we also want to add our own listeners for triggering the notifications (again, we need a decoupling between registry and protocol currently).
With PSR-14 the
EventDispatcherInterfaceused in theRegistrydoesn't offer us to register our own listeners after a user injected it viaBuilder::setEventDispatcher()Session ID State
I need to call the
Protocol::sendNotification(...)from the listeners, but didn't have the session at hand - and there is basically no state in the Protocol - which is great, but we don't have a service concept to retrieve the session - it is only state in the transport or arguments looped through half the SDK.I went for introducing the session as state in the
Protocol- needs better null-checks tho, but wanted to check with @CodeWithKyrian first - the other option would be to keep the transport as state, since it also has the session already as state.WDYT?
edit: this currently messes up the session handling within a fiber intermediate client request.